logic: Logic overrides the instance variable even when explicitly set in forward call - should preserve user's explicit choice. Should the mask type check override an explicitly passed bottom_right_diagonal parameter, or only apply when it's None?

logic: Same logic issue as above - mask type check overrides explicit parameter values

Could you add a couple of SWA tests to the CP tests as well? I think it's just a matter of replacing (left,0) with (left, right) and test them out. Thanks!

Critical: Variable name mismatch causing incorrect backend selection

This line sets use_flash_attention instead of use_flash_attention_2, which was the intended target based on the conditional check on line 906. This causes FlashAttention to be incorrectly disabled when it should only disable FlashAttention 2.

Impact: This bug causes FlashAttention to be incorrectly disabled for sliding window attention with cross-attention scenarios where bottom_right_diagonal=False and max_seqlen_q != max_seqlen_kv. The code should only disable FlashAttention 2 in this case, not all FlashAttention backends.

Critical: Variable name mismatch causing incorrect backend selection

Similar to line 911, this line sets use_flash_attention instead of use_flash_attention_2. The conditional check on line 933 verifies not bottom_right_diagonal and max_seqlen_q != max_seqlen_kv for FlashAttention 2 with ALiBi, but then incorrectly disables all FlashAttention instead of just FlashAttention 2.

Impact: This causes FlashAttention to be incorrectly disabled for ALiBi attention with cross-attention where bottom_right_diagonal=False and max_seqlen_q != max_seqlen_kv. Only FlashAttention 2 should be disabled in this scenario.

Logic issue: Explicit bottom_right_diagonal parameter is unconditionally overridden

When a user explicitly passes bottom_right_diagonal=False and attn_mask_type is "causal_bottom_right", the code first sets it to False on line 1100, but then immediately overrides it to True on line 1105. This means the explicit parameter value is ignored.

Problem scenario:

# User explicitly requests bottom_right_diagonal=False
forward(..., attn_mask_type="causal_bottom_right", bottom_right_diagonal=False)

# Line 1100: bottom_right_diagonal = False  (respects explicit value)
# Line 1105: bottom_right_diagonal = True   (overrides explicit value!)

The condition on line 1101 checks bottom_right_diagonal is None, which will be False after line 1100, but then the or clause still forces it to True.

Suggested fix: The logic should respect explicit values and only set defaults when bottom_right_diagonal is None:

if attn_mask_type in {"causal", "padding_causal"}:
    if bottom_right_diagonal is None:
        bottom_right_diagonal = False
elif bottom_right_diagonal is None:
    # Only set to True if not explicitly specified
    if attn_mask_type in {"causal_bottom_right", "padding_causal_bottom_right"}:
        bottom_right_diagonal = True
    else:
        bottom_right_diagonal = True  # default for other mask types

This same pattern appears in multiple locations and should be fixed consistently throughout the codebase.

Could we add some explanation in the init or fwd docstring that bottom_right_diagonal will be forced to True if the mask type has bottom_right in it. Just so users (and Greptile) don't get confused. It helps remove this comment from Greptile as well.

You're absolutely right! The current behavior where bottom_right_diagonal gets overridden based on the mask type should be clearly documented. This would help users understand when their explicit parameter values will be respected versus when they'll be overridden.

Looking at the current logic, I can see that:

1. Line 1100: If mask type is "causal" or "padding_causal", it's forced to False
2. Lines 1101-1105: If it's None OR mask type contains "bottom_right", it's forced to True

Here's a suggestion to add clarity to the docstring:

This documentation makes it clear that the parameter can be overridden, helping users understand the actual behavior and avoiding confusion about when explicit values are respected.

Logic issue: Explicit bottom_right_diagonal parameter is unconditionally overridden

This has the same logic flaw as in dot_product_attention.py. When a user explicitly passes bottom_right_diagonal=False with attn_mask_type="causal_bottom_right", the explicit value gets overridden.

The code flow:

1. Line 752: Sets bottom_right_diagonal = False for causal masks
2. Line 753-757: Immediately overrides to True if mask type contains "bottom_right"

This means explicit parameter values are ignored. The logic should only set defaults when bottom_right_diagonal is None, not override explicit values.

Logic issue: Explicit bottom_right_diagonal parameters are unconditionally overridden

This code has the same logic flaw as seen in other files. Both bottom_right_diagonal (lines 777-783) and enc_dec_bottom_right_diagonal (lines 787-793) suffer from the same issue where explicit parameter values get overridden.

When a user explicitly passes these parameters with specific mask types, the explicit values are ignored because:

1. Lines 778, 788: Set to False for causal masks
2. Lines 779-783, 789-793: Immediately override to True if mask type contains "bottom_right"

The logic should respect explicit values and only set defaults when the parameters are None.

Is this change intended, or it's for debugging? Would this fix the printing issue we talked about before, where the unit test wouldn't print the selection logic from this for loop?

apparently, its use here is a bit problematic and so removing it helps print the information. (I vaguely remember that you suggested me to remove it and then I checked logging documentation and you were right)

 with logging_context(highest_level=AttentionLogging._log_level):

Why is alibi_slopes_shape is None branch removed here?

The encapsulating if condition makes sure that this branch is always false

if(
...
and (alibi_slopes_shape is not None)
):

style: Documentation inconsistency: forward function documents default as None but backward documents True

logic: Critical: Hardcoded parameter instead of proper API parameter

This hardcoded true value should be replaced with a proper bottom_right_diagonal parameter passed from the function signature. The current implementation bypasses the intended parameter passing mechanism.

Issue: The function fused_attn_fp8_fwd_impl_v1 should accept bottom_right_diagonal as a parameter and pass it here, rather than hardcoding true. This breaks the configurability that the PR is supposed to add. Should this hardcoded true be replaced with a bottom_right_diagonal parameter that gets passed through the function signature?

logic: Critical: Hardcoded parameter instead of proper API parameter

Similar to the forward pass, this hardcoded false value should be replaced with a proper bottom_right_diagonal parameter passed from the function signature.

Issue: The function fused_attn_fp8_bwd_impl_v1 should accept bottom_right_diagonal as a parameter and pass it here. The hardcoded false suggests different behavior for backward pass, but this should still be configurable. Should this hardcoded false be replaced with a bottom_right_diagonal parameter, and why is it different from the forward pass value?